feat(autoware_carla_interface): publish CARLA traffic-light states, matched to the map by position - #13327
Conversation
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
9d538e5 to
706b02c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d538e50b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…atched to the map by position Bridge the CARLA server's traffic-light states into Autoware's perception output so a CARLA closed loop can run without camera-based recognition. The key problem is associating a CARLA traffic light with an Autoware `traffic_light_group_id` (a `traffic_light` regulatory-element id in the lanelet2 map). Instead of assuming the CARLA OpenDRIVE signal id equals the regulatory-element id (true only for maps auto-generated from the same OpenDRIVE) or hand-writing an id table, the bridge discovers the mapping geometrically: each CARLA light head is matched to the nearest lanelet2 light head and its state is published under every regulatory element that references that head. This works for hand-authored / Vector Map Builder maps too. - New `modules/traffic_light_matcher.py`: parses the lanelet2 `.osm` directly (reads `local_x`/`local_y`, i.e. the map frame; no lanelet2/projector dependency), keys physical heads by their `refers` way, and matches CARLA heads conservatively (distance threshold + a disjoint-group ambiguity ratio), dropping and logging ambiguous / too-far lights rather than guessing. - `carla_ros.py`: publishes `TrafficLightGroupArray` on /perception/traffic_light_recognition/traffic_signals, aggregated per group. - `carla_autoware.py`: `force_green` freezes all lights green for camera-less runs. - Parameters grouped under the `traffic_light.` namespace; resolution order is id-map override -> position match -> OpenDRIVE-id fallback. - Unit tests for the matcher; README documents the feature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
…matching
- Use the resolved map origin (`_current_map_origin()`) instead of the raw
`map_origin_x/y` parameters when placing CARLA light heads in the map frame,
so georeferenced maps (origin derived from the OpenDRIVE geoReference in
`on_world_ready`, parameters left at zero) match correctly instead of falling
outside the distance threshold and publishing nothing. (P1)
- Treat a candidate head as a genuine alternative for the ambiguity test unless
its group set is exactly equal to the winner's, replacing the `isdisjoint`
check. Overlapping-but-unequal sets (e.g. {500, 501} vs {501}) would otherwise
be accepted by arbitrary ranking and publish a missing or spurious group. Adds
a regression test. (P2)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
46c8693 to
da0463e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13327 +/- ##
==========================================
- Coverage 20.48% 20.47% -0.02%
==========================================
Files 1948 1948
Lines 137253 137024 -229
Branches 48956 48863 -93
==========================================
- Hits 28123 28050 -73
+ Misses 86336 86251 -85
+ Partials 22794 22723 -71
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da0463e0ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@hakuturu583 Hi, thanks for the PR. Could please help solve the failed CodeScene Check? |
…cene
Address the second review round:
- traffic_light.id_map now accepts several group ids per OpenDRIVE signal id,
formatted `opendrive_id:group_id[|group_id...],...` (repeated keys merge). A
physical head shared by multiple regulatory elements can therefore be pinned
to all of them via one override, matching the position matcher's shared-head
behaviour, instead of only ever recovering a single group. (Codex P2)
- Reduce complexity flagged by the CodeScene gate by extracting helpers so the
hot methods stay flat and short:
- traffic_light_matcher: split `load_map_traffic_lights` into node/way/relation
parsers (Complex Method / Bumpy Road) and pull the per-head decision out of
`match_traffic_lights` into `_classify_head` (Large Method).
- carla_ros: split `_resolve_traffic_light_groups` into `_apply_id_map_override`
/ `_match_actors_to_map` / `_fallback_opendrive_groups`; the override parser
is now the pure, unit-tested `parse_id_map_override`.
- carla_autoware: fold the enable check into `_force_green_traffic_lights` so
`load_world` keeps a single unconditional call and gains no branch.
- Extend unit tests for the multi-id override parsing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
|
@Max-Bin thanks for the review! I've addressed the failing CodeScene gate in c52ece1 by refactoring the methods it flagged (rather than suppressing them):
The same commit also addresses the Codex P2 comment (id_map now supports multiple group ids per signal). Tests still pass and black is clean; CI should re-run the CodeScene check on the new commit. |
Follow-up to the previous refactor, targeting the two rules still flagged on the traffic-light matcher: - Extract `_add_relation_heads` so `load_map_traffic_lights` is a single flat loop with no nested conditional block (clears Bumpy Road Ahead). - Pass the per-run matching state as one `_MatchContext` namedtuple instead of a seven-parameter list, so `_classify_head(head, ctx)` takes two arguments (clears Excess Number of Function Arguments). No behavioural change; tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
…own table) On top of the bot autofix, finish the parts it could not: - Capitalize the first word of the parser-helper docstrings (flake8 / pydocstyle D403). - Order the traffic_light_matcher imports as isort (profile=black) expects. - Remove the literal `|` from the traffic_light.id_map row of the README parameter table (it split the cell and broke the column count; the bot's --fix left it mangled). The `|`-separated multi-id syntax stays documented in prose. markdownlint and prettier now pass on the table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
… the off state
Review follow-ups on the CARLA traffic-light bridge:
- traffic_light.id_map no longer lets a typo through split()/int(). A
malformed entry ("12" with no ':', "12:" with no group id, or a
non-integer id) is skipped with a warning naming the offending entry
instead of raising on the first simulation tick or silently overriding
a light with an empty group list; the other entries still apply.
Reporting goes through an optional on_invalid callback so the parser
stays ROS-free and unit-testable.
- The ambiguity guard now rejects an exact tie. With two heads of
different signals at the same point both distances are 0, so
"nearest > ratio * second" degenerates to 0 > 0 and the winner came
down to the order the ways appear in the .osm; a tie has no winner and
is now reported like any other ambiguous match.
- CARLA's TrafficLightState.Off is a known state (the signal is dark),
but it was published as an UNKNOWN-color lamp with status SOLID_ON,
i.e. "unknown color, lit". It now maps to status SOLID_OFF, and only a
state the bridge cannot interpret stays UNKNOWN/UNKNOWN.
Tests cover the malformed-entry cases and the coincident-head tie.
Signed-off-by: Masaya Kataoka <ms.kataoka@gmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MCAQHBrcSCmJoKn5Xi7s3k
…pell check "unsignalized" is not in the Autoware spell-check dictionary and failed spell-check-differential. Say the same thing in words the dictionary knows; no behavior change. Signed-off-by: Masaya Kataoka <ms.kataoka@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCAQHBrcSCmJoKn5Xi7s3k
…uity test CodeScene flagged the previous commit for a Complex Method (_parse_id_map_entry: two try/except blocks and a loop in one function) and a Complex Conditional (_classify_head's compound ambiguity check). Pull the pieces out instead of suppressing them: _parse_id (one integer id) and _parse_id_map_groups (the group-id list) leave the entry parser as a separator check plus two calls, and _is_ambiguous names the runner-up test as a guarded sequence of single conditions. No behavior change; the matcher tests still pass. Signed-off-by: Masaya Kataoka <ms.kataoka@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCAQHBrcSCmJoKn5Xi7s3k
Description
Bridges the CARLA server's traffic-light states into Autoware's perception output, so a CARLA
closed loop can run without camera-based traffic-light recognition.
This reworks the approach explored in #13309. The core problem is how a CARLA traffic light is
associated with an Autoware
traffic_light_group_id(the id of atraffic_lightregulatoryelement in the lanelet2 map). Rather than assume the CARLA OpenDRIVE signal id equals the
regulatory-element id — which only holds for maps auto-generated straight from the same OpenDRIVE —
or require a hand-written id table, the bridge discovers the mapping geometrically: each CARLA
light head is matched to the nearest lanelet2 light head and its state is published under every
regulatory element that references that head. This makes the feature work for hand-authored / TIER
IV Vector Map Builder maps, whose regulatory-element ids do not correspond to the OpenDRIVE signal
ids.
What's added
Launch arguments (all off / empty by default, so existing behavior is unchanged). Each launch
argument carries the same name as the node parameter it sets, so the settings stay grouped under
the
traffic_light.namespace inros2 param list:traffic_light.publishautoware_perception_msgs/TrafficLightGroupArrayon/perception/traffic_light_recognition/traffic_signalsevery tick.traffic_light.force_greentraffic_light.map_path.osmused for position matching.traffic_light.match_distance5.0m).traffic_light.match_ratio0.6).traffic_light.id_mapopendrive_id:group_id[|group_id...],...override that pins a light, taking precedence over matching.How the matching works
modules/traffic_light_matcher.py(ROS-free, unit-tested):.osmdirectly as XML, reading each node'slocal_x/local_ytags — theAutoware map-frame coordinates (for an MGRS map, exactly the easting/northing the map loader
produces). This avoids depending on the lanelet2 C++ regulatory-element registration and
sidesteps any projector mismatch between this process and the map loader.
refersway, commonly shared by several regulatory elements (one perapproaching lane / stop line), so each head carries the set of group ids that reference it; a
matched CARLA light publishes its state under all of them.
get_light_boxes()centresvia the existing
carla_location_to_ros_point(map_origin_x, map_origin_y)transform.resolving to a different signal is nearly as close (
nearest > ratio * second, the "lightacross the intersection" case), is exactly as close (a tie has no winner, so the
.osmordermust not decide it), or nothing is within
match_distance, the light is left unpublished andlogged as ambiguous / too-far — never guessed. The startup log prints a match report
(
N matched, M ambiguous, K too far); reported lights can be pinned viatraffic_light.id_map.Resolution precedence per light:
traffic_light.id_mapoverride → position match(
traffic_light.map_path) → OpenDRIVE-id-as-group-id fallback (no map path).A malformed
traffic_light.id_mapentry (no:, a non-integer id, or no group id after the:)is skipped with a warning naming it, so a typo neither takes the bridge down on the first tick nor
silently overrides a light with an empty group list.
Published states
Red/Yellow/Greenmap toRED/AMBER/GREENwith statusSOLID_ON; CARLA's known-darkOffmaps to statusSOLID_OFF, and only a state the bridge cannot interpret is published asUNKNOWN/UNKNOWN.How was this PR tested?
test/test_traffic_light_matcher.py— unit tests covering shared-head group expansion, isolatedmatches, the ambiguity guard (the disjoint-signal midpoint and the coincident-head tie that must
be dropped, and the close same-group / clear-winner neighbours that must not), too-far,
.osmparsing, and the
traffic_light.id_mapparser including its malformed-entry handling.realistic alignment (~0.3 m) ~97 % of heads match, and genuinely-ambiguous sub-metre distinct
signals are reported rather than mis-assigned.
black(line-length 100) clean; dotted parameter names verified to declare / resolve underrclpy.
traffic_light.publish(and, for matching,traffic_light.map_path)is set.